Skip to content

Conversation

Prabhuk
Copy link
Contributor

@Prabhuk Prabhuk commented Sep 22, 2025

static is not allowed inside constexpr functions in C++ versions below
23. This is causing a build error when compiled with GCC and warning
when compiled with Clang. This patch fixes this.

`static` is not allowed inside constexpr functions in C++ versions below
23. This is causing a build error when compiled with GCC and warning
when compiled with Clang. This patch fixes this.
@Prabhuk Prabhuk requested a review from a team as a code owner September 22, 2025 19:26
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Sep 22, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 22, 2025

@llvm/pr-subscribers-libcxx

Author: Prabhu Rajasekaran (Prabhuk)

Changes

static is not allowed inside constexpr functions in C++ versions below
23. This is causing a build error when compiled with GCC and warning
when compiled with Clang. This patch fixes this.


Full diff: https://github.com/llvm/llvm-project/pull/160180.diff

1 Files Affected:

  • (modified) libcxx/include/__algorithm/make_heap.h (+2-1)
diff --git a/libcxx/include/__algorithm/make_heap.h b/libcxx/include/__algorithm/make_heap.h
index 8cfeda2b59811..c592656374dac 100644
--- a/libcxx/include/__algorithm/make_heap.h
+++ b/libcxx/include/__algorithm/make_heap.h
@@ -36,7 +36,8 @@ __make_heap(_RandomAccessIterator __first, _RandomAccessIterator __last, _Compar
   using __diff_t     = __iter_diff_t<_RandomAccessIterator>;
   const __diff_t __n = __last - __first;
 
-  static const bool __assume_both_children = is_arithmetic<__iter_value_type<_RandomAccessIterator> >::value;
+  _LIBCPP_CONSTEXPR_SINCE_CXX14 const bool __assume_both_children =
+      is_arithmetic<__iter_value_type<_RandomAccessIterator> >::value;
 
   // While it would be correct to always assume we have both children, in practice we observed this to be a performance
   // improvement only for arithmetic types.

Prabhuk and others added 2 commits September 22, 2025 16:34
Drop static and constexpr for __assume_both_children. Simply leave it to be plain `const`.

Co-authored-by: A. Jiang <de34@live.cn>
@Prabhuk
Copy link
Contributor Author

Prabhuk commented Sep 23, 2025

@frederick-vs-ja Can you PTAL at the updated patch? This issue is breaking our downstream builders. Thank you.

@Prabhuk Prabhuk requested a review from frobtech September 23, 2025 18:52
Copy link
Contributor

@frobtech frobtech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@frederick-vs-ja frederick-vs-ja merged commit 59b4621 into llvm:main Sep 23, 2025
78 checks passed
@Prabhuk
Copy link
Contributor Author

Prabhuk commented Sep 23, 2025

Thank you @frederick-vs-ja

@Prabhuk Prabhuk deleted the libcxx_static_in_constexpr branch September 25, 2025 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants